Include constant-time analysis framework#2449
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| steps: | ||
| - name: Example Interactive Inputs Step | ||
| id: interactive-inputs | ||
| uses: boasiHQ/interactive-inputs@v2 |
| steps: | ||
| - name: Example Interactive Inputs Step | ||
| id: interactive-inputs | ||
| uses: boasiHQ/interactive-inputs@v2 |
Signed-off-by: Pablo Gutiérrez <pablogf@uma.es>
|
Coverage is 82.191% — pablo-gf:ct-tooling into open-quantum-safe:main. No base build found for |
bhess
left a comment
bhess
left a comment
There was a problem hiding this comment.
Thank you @pablo-gf for this work on improving the constant‑time tooling in liboqs, this is an important step towards more exhaustive ct-testing in liboqs.
I’ve left a few initial comments inline.
I would be happy to walk through the details of any of these features either here or during a status call.
That would be excellent. @dstebila / @RodriM11: could we allocate some time in an upcoming status call for @pablo-gf to walk us through this contribution? I think a walkthrough would be very valuable.
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| interactive-inputs: |
There was a problem hiding this comment.
My understanding is that this job requires interactive input to select the algorithms. Is that correct? If so, what is the intended usage, and can it be configured to run non‑interactively (e.g., for inclusion in the weekly CI runs)?
| display: Select the algorithm(s) to execute valgrind-varlat constant-time testing on | ||
| type: multiselect | ||
| choices: | ||
| - "BIKE-L1" |
There was a problem hiding this comment.
My concern with this list is maintainability: if the set of algorithms in liboqs changes, the list would need to be updated manually. Is there a way to generate this list dynamically based on the algorithms currently available?
| @@ -0,0 +1,396 @@ | |||
| // SPDX-License-Identifier: MIT | |||
There was a problem hiding this comment.
The test_sig.c / test_kem.c files and the corresponding CMakeLists.txt appear to be largely copied from the versions in the /tests directory. Could we avoid duplicating these files and instead reuse the originals with additional macro definitions (e.g., to disable signature verification during ct-testing)? Or is there a specific reason why maintaining separate copies is necessary here?
| @@ -0,0 +1,15 @@ | |||
| fun:PQCP_MLKEM_NATIVE_MLKEM*_C_poly_decompress_d* | |||
There was a problem hiding this comment.
I really like this approach, having an allowlist mechanism similar to the Valgrind one makes a lot of sense.
| INSTALL_PREFIX="$PWD/valgrind_varlat" | ||
|
|
||
| echo "Cloning Valgrind's source code" | ||
| git clone git://sourceware.org/git/valgrind.git valgrind_varlat_src> /dev/null 2>&1 || true |
There was a problem hiding this comment.
The statements with || true would silently fail.
| done | ||
|
|
||
| # Create backup files of the original tests files | ||
| mv "$LIBOQS_DIR/tests/CMakeLists.txt" "$LIBOQS_DIR/tests/CMakeLists.txt.bak" |
There was a problem hiding this comment.
This looks brittle since it makes assumptions about the liboqs file structure. IMO avoiding replacing the original files and instead work CT-specific macros in the original files would be more robust.
5 participants
This PR includes the constant-time testing framework developed throughout the course of the LFX Mentorship project supervised by @bhess. These are the main features of this framework:
I would be happy to walk through the details of any of these features either here or during a status call. You can also find detailed guides in the documentation files I’ve added under
tests/ct_toolingand its subdirectories. Feel free to throw any feedback or comments so that we can move this from draft to "ready for review"!AI assistance was used to generate documentation and minor editing. The resulting code was reviewed, edited and verified for accuracy.